Add an edit deployment API #2515
Open
+961
−174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| existing: DeploymentId, | ||
| }, | ||
| #[error("an update deployment operation must provide an endpoint with the same services and handlers. The update tried to remove the services {0:?}")] | ||
| #[code(restate_errors::META0006)] |
There was a problem hiding this comment.
I would define a new error code for errors related to update. META0006 seems wrong as it talks about adding deployments. In the error code you can describe in a more lengthy manner what to do
Comment on lines
+260
to
+265
| pub fn update_deployment( | ||
| &mut self, | ||
| deployment_id: DeploymentId, | ||
| deployment_metadata: DeploymentMetadata, | ||
| services: Vec<endpoint_manifest::Service>, | ||
| ) -> Result<(), SchemaError> { |
There was a problem hiding this comment.
Maybe it's me, but I couldn't find a check that verifies the supported service protocol range is the same. This is definitely the most important thing to check, as there is a super high change that if they don't match, the invocation will get broken/corrupted.
Comment on lines
+301
to
+313
| let handlers = DiscoveredHandlerMetadata::compute_handlers( | ||
| service | ||
| .handlers | ||
| .into_iter() | ||
| .map(|h| { | ||
| DiscoveredHandlerMetadata::from_schema( | ||
| service_name.as_ref(), | ||
| service_type, | ||
| h, | ||
| ) | ||
| }) | ||
| .collect::<Result<Vec<_>, _>>()?, | ||
| ); |
There was a problem hiding this comment.
I think you're missing somewhere to preserve the private handler flag? Maybe add a unit test to check that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds an alternative to force which allows you to update an existing deployment ID to have a new arn/uri/etc, including adding services and handlers if necessary. For break glass operations to get you out of a jam with failing invocations.
We do allow this operation to move existing deployments to point to the same URI as other deployments, an invariant we previously kept. However, in this case, we will not allow further force deploys against that URI until some deployments are removed to make it unambigious which deployment ID the force should subsume
The recommended workflow is as follows:
If you notice failing invocations after you've already deployed some other change, because the previous deployment hasn't drained, the workflow is as follows: